Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: expose --generate-hardware-config nixos-facter thru TF #408

Merged
merged 5 commits into from
Oct 21, 2024
Merged

feat: expose --generate-hardware-config nixos-facter thru TF #408

merged 5 commits into from
Oct 21, 2024

Conversation

KiaraGrouwstra
Copy link
Contributor

add var.nixos_facter_path to TF modules install / all-in-one.

c.f. #407, which handled --generate-hardware-config nixos-generate-config.

P.S.: this PR builds upon that one, figuring it would be more elegant to use the elif than have two separate if statements.

Mic92
Mic92 reviewed Oct 16, 2024
View reviewed changes
flake.nix Outdated Show resolved Hide resolved
@KiaraGrouwstra KiaraGrouwstra force-pushed the tf-nixos-facter branch 2 times, most recently from 44212ac to b8d1b40 Compare October 20, 2024 08:19
@KiaraGrouwstra
feat: expose --generate-hardware-config nixos-facter thru TF
9462024 
add `var.nixos_facter_path` to TF modules `install` / `all-in-one`.

c.f. nix-community#407, which handled `--generate-hardware-config
nixos-generate-config`.
@Mic92
Copy link
Member

Mic92 commented Oct 20, 2024 •
edited
Loading

We don't have a integration test for terraform. @KiaraGrouwstra could you test this one more time to make sure nothing is broken. Otherwise I would need to run a test later.

@KiaraGrouwstra
Copy link
Contributor Author

@Mic92

We don't have a integration test for terraform. @KiaraGrouwstra could you test this one more time to make sure nothing is broken. Otherwise I would need to run a test later.

tho in retrospect separate from TF, i pushed an update to the nixos-anywhere bash script to ensure any directories of hardware configuration files would be created, if needed.

that made it a bit more robust, and had it go thru for me. as in #407, in my use i am still checking if pathExists again using (lib.optionalAttrs (lib.pathExists reportPath) { config.facter = { inherit reportPath; }; }). that is, if using flakes + git, the user is currently still expected to manually stage the generated file to git for the nixos config to actually pick it up.
(that said, that particular weakness too i think seems not specific to TF.)

@Mic92
Copy link
Member

Mic92 commented Oct 21, 2024

The git add we already do:

nixos-anywhere/src/nixos-anywhere.sh

Line 458 in 3fb834d

git add --intent-to-add --force -- "$hardwareConfigPath" >/dev/null 2>&1 || true

@Mic92
Copy link
Member

Mic92 commented Oct 21, 2024

@mergify queue

@mergify Mergify
Copy link
Contributor

mergify bot commented Oct 21, 2024 •
edited
Loading

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 51d347d

@mergify mergify bot merged commit 51d347d into nix-community:main Oct 21, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mic92 Mic92 Mic92 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@KiaraGrouwstra @Mic92